Automate configuration metadata for @ConfigurationProperties modules - #16047
Automate configuration metadata for @ConfigurationProperties modules#16047jamesfredley wants to merge 10 commits into
Conversation
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
There was a problem hiding this comment.
Pull request overview
Introduces a new build-time pipeline to generate Spring Boot configuration metadata from compiled Groovy/Java @ConfigurationProperties classes, merging in curated additional-spring-configuration-metadata.json overlays and wiring the results into the docs config reference generation.
Changes:
- Adds
grails-configuration-metadatacompiler module with a Groovy SEMANTIC_ANALYSIS AST transformation to embed deterministic metadata payloads in compiled Groovy configuration classes. - Adds a Gradle build-logic plugin (
org.apache.grails.buildsrc.configuration-metadata) that scans compiled bytecode (ASM), merges curated overlays, and publishes a single standardMETA-INF/spring-configuration-metadata.jsonresource per jar. - Migrates existing curated metadata to
additional-spring-configuration-metadata.json, relocates CORS metadata ownership to URL Mappings, and expands the docs config-reference pipeline inputs.
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle | Includes new grails-configuration-metadata module in the multi-project build. |
| gradle/publish-root-config.gradle | Publishes the new grails-configuration-metadata module. |
| dependencies.gradle | Adds ASM to the build BOM dependencies for bytecode scanning. |
| build-logic/plugins/build.gradle | Adds ASM dependency and registers the new configuration metadata Gradle plugin. |
| build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy | Implements bytecode scanning + overlay merge + deterministic metadata output task wired into processResources. |
| build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPluginSpec.groovy | TestKit coverage for clean/incremental/edit/deletion/overlay/duplicates/no-overlay behavior. |
| grails-configuration-metadata/build.gradle | New compiler module build definition and test dependencies. |
| grails-configuration-metadata/src/main/groovy/org/apache/grails/configuration/metadata/ConfigurationMetadataTransformation.groovy | Groovy AST transform embedding per-class metadata payloads (constant defaults only). |
| grails-configuration-metadata/src/main/resources/META-INF/services/org.codehaus.groovy.transform.ASTTransformation | Registers the global AST transformation. |
| grails-configuration-metadata/src/test/groovy/org/apache/grails/configuration/metadata/ConfigurationMetadataTransformationSpec.groovy | Unit tests validating payload shape, defaults policy, and reserved-field collision handling. |
| grails-cache/build.gradle | Applies the new configuration metadata build plugin. |
| grails-cache/src/main/resources/META-INF/additional-spring-configuration-metadata.json | Adds curated cache metadata overlay for merge with generated metadata. |
| grails-databinding/build.gradle | Applies the new configuration metadata build plugin. |
| grails-databinding/src/main/resources/META-INF/additional-spring-configuration-metadata.json | Adds curated data binding metadata overlay. |
| grails-views-gson/build.gradle | Applies the new configuration metadata build plugin. |
| grails-views-gson/src/main/resources/META-INF/additional-spring-configuration-metadata.json | Adds curated JSON Views metadata overlay. |
| grails-views-markup/build.gradle | Applies the new configuration metadata build plugin. |
| grails-web-url-mappings/build.gradle | Applies the new configuration metadata build plugin. |
| grails-web-url-mappings/src/main/resources/META-INF/additional-spring-configuration-metadata.json | Adds curated URL Mappings/CORS metadata overlay (migrated from web-core). |
| grails-web-core/src/main/resources/META-INF/spring-configuration-metadata.json | Removes CORS group/properties now owned by URL Mappings. |
| grails-doc/src/en/guide/conf/config.adoc | Documents how Grails produces/overlays configuration metadata and its defaults policy. |
| grails-doc/build.gradle | Adds migrated module jars as inputs to the configuration reference generation pipeline. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16047 +/- ##
==================================================
- Coverage 51.4910% 51.4794% -0.0117%
- Complexity 17762 17793 +31
==================================================
Files 2039 2040 +1
Lines 95537 95681 +144
Branches 16571 16610 +39
==================================================
+ Hits 49193 49256 +63
- Misses 39037 39084 +47
- Partials 7307 7341 +34
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: b792a3f Learn more about TestLens at testlens.app. |
|
We had discussed creating a custom solution and not requiring ConfigurationProperties in the meeting, why the reversal here now? |
jdaugherty
left a comment
There was a problem hiding this comment.
I had AI review this and then iterated with the review. I think the @Delegate finding is rather important. Review follows.
Read through the compiler module, the Gradle plugin and the five module migrations, then ran generateConfigurationMetadata for all five modules plus :grails-doc:generateConfigReference on this branch to diff the actual output. The mechanism works: the ASM path and the Groovy payload path both produce correct metadata for Java and Groovy config classes, and cache / data binding / JSON views / CORS come through with no loss. The TestKit spec covers clean, incremental, edit, delete and overlay behaviour well.
One thing I would want resolved before this goes in: the Application Properties reference regresses. 33 of 187 rows come out with an empty description and an empty default (24 grails.views.markup.*, 9 newly inferred grails.views.json.*). Every other row in that file has a description, so this PR introduces all 33.
The remaining comments are about how quietly the generated half can drift from what Boot actually binds (the @Delegate blind spot and the isNested heuristic), how thinly the transform itself is covered, and a question about publishing the compiler module.
This review is on the implementation as written; it does not address the separate open question about whether the approach should require @ConfigurationProperties.
| project(':grails-data-mongodb').tasks.named('jar'), | ||
| project(':grails-views-gson').tasks.named('jar') | ||
| project(':grails-views-gson').tasks.named('jar'), | ||
| project(':grails-views-markup').tasks.named('jar'), |
There was a problem hiding this comment.
Wiring these two jars into generateConfigReference publishes the generated metadata straight into the Application Properties reference, and grails-views-markup has no curated overlay. Generating the reference from this branch:
- 33 of 187 rows now have an empty Description and an empty Default: all 24
grails.views.markup.*, plus the 9 newly inferredgrails.views.json.*entries (baseTemplateClass,cache,enableReloading,extension,packageImports,packageName,staticImports,templatePath,useAbsoluteLinks). Every pre-existing row in that file has a description, so all 33 blanks are new. - The markup rows land inside the existing "Views & GSP" section, interleaved with fully documented
grails.views.gsp.*rows, so they read as holes in an otherwise complete table.
Every one of these is settable, so every one needs documenting. Can we add grails-views-markup/src/main/resources/META-INF/additional-spring-configuration-metadata.json with a group description plus a description and default for each of the 24, and descriptions for the 9 new JSON Views entries, before these jars feed the reference?
| * deferred to the Gradle task so incremental Groovy compilation never writes shared output. | ||
| */ | ||
| @CompileStatic | ||
| @GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS) |
There was a problem hiding this comment.
SEMANTIC_ANALYSIS runs before @Delegate composes (DelegateASTTransformation is CANONICALIZATION), so delegate-generated setters are invisible to collectSetterProperties. CORS is exactly that case: running :grails-web-url-mappings:generateConfigurationMetadata on this branch, the generated half contributes only grails.cors.enabled and grails.cors.mappings. allowedOrigins, allowedMethods, allowedHeaders, exposedHeaders, maxAge and allowCredentials — all bindable through the @Delegate GrailsDefaultCorsConfiguration and Spring's CorsConfiguration setters — survive only because the new overlay lists them by hand.
That is a reasonable limitation to accept, but today it fails silently: add a delegated property and the metadata simply will not have it, with nothing in the build to say so. Can we state the limitation in the class documentation, and ideally have the transform warn (or the task fail) when a @ConfigurationProperties class carries a @Delegate field, so whoever adds one is told to extend the overlay?
| node.interfaces.each { ClassNode interfaceNode -> collectSetterProperties(interfaceNode, bindable, visited) } | ||
| } | ||
|
|
||
| private static boolean isNested(ClassNode type) { |
There was a problem hiding this comment.
isNested treats every type outside java.*/groovy.* as a nested configuration group, and collectSetterProperties walks superclasses and interfaces without applying that filter at all — it only stops at Object. The two hardcoded names on line 174 are what keeps that from exploding today: JsonViewConfiguration and MarkupViewConfiguration implement the GenericViewConfiguration trait, which extends GrailsApplicationAware, so without the setGrailsApplication exclusion the transform would recurse into GrailsApplication.
Boot's own processor keys nesting off inner classes and @NestedConfigurationProperty rather than a package-name heuristic, and the ASM path in this PR is already narrower — it nests only into types found in the same module's output (models[property.rawType()]). Could the Groovy path use the same restriction, and honour @NestedConfigurationProperty? As written, the next config class with a setter taking a framework or third-party bean type silently emits a nested group tree, and the remedy will be another name added to that blocklist.
| id 'org.apache.grails.buildsrc.properties' | ||
| id 'org.apache.grails.buildsrc.dependency-validator' | ||
| id 'org.apache.grails.buildsrc.compile' | ||
| id 'org.apache.grails.buildsrc.publish' |
There was a problem hiding this comment.
Applying buildsrc.publish (with the matching publish-root-config.gradle entry) puts this module on Maven Central, and because grails-bom/base adds every published subproject as a constraint it lands in grails-bom too. It is also the only new module that deliberately skips gradle/docs-config.gradle, so it ships as a published artifact with no API docs. It is compileOnly on the five framework modules, so it never reaches their POMs, and nothing outside this build can use it without the unpublished build-logic plugin.
It also registers a global transform via META-INF/services/org.codehaus.groovy.transform.ASTTransformation. Once published, anyone who ends up with it on a compile classpath silently gets __grailsConfigurationMetadata injected into their own @ConfigurationProperties classes, and a hard compile error if they happen to declare a field with that name. Is there a consumer that needs the artifact? If not, dropping the publish/sbom/vulnerability-scan plugins and the publishedProjects entry keeps it internal to the build.
Separately, compileOnly 'org.springframework.boot:spring-boot' on line 39 looks unused — the transform matches ConfigurationProperties and ConstructorBinding by name string and imports nothing from Spring.
| .forEach { java.nio.file.Path path -> | ||
| ClassModel model = GenerateConfigurationMetadataTask.readClass(Files.readAllBytes(path)) | ||
| ClassModel previous = models.put(model.name, model) | ||
| if (previous != null && previous != model) { |
There was a problem hiding this comment.
ClassModel does not implement equals, so previous != model is identity comparison and is always true for two distinct instances. This reduces to "fail on any duplicate class name across the input directories", which is not what the guard reads as. Either give ClassModel value equality (or compare prefix/payload) so a genuinely identical duplicate is tolerated as intended, or drop the second condition so the code says what it does.
| merged.keySet().sort().collect { String name -> merged[name] } | ||
| } | ||
|
|
||
| private static Map<String, Object> indexByName(List source, String category, String sourceName) { |
There was a problem hiding this comment.
indexByName enforces name uniqueness per category. That is right for properties, but Boot's format allows repeated group names distinguished by sourceType/sourceMethod, and generate() emits one group per @ConfigurationProperties class (line 128) plus one per nested holder. Two config classes sharing a prefix in the same module, or a nested prefix reachable from two classes, would fail the build with Conflicting generated groups metadata for '...' on metadata that is legal. Worth keying groups on name + sourceType, or merging instead of rejecting.
| "description": "CORS" | ||
| }, | ||
| { | ||
| "name": "grails.mime", |
There was a problem hiding this comment.
With CORS moved out, this file still carries 10 groups and 24 properties for prefixes spanning URL mappings, content negotiation, static resources and scaffolding. Not a blocker for this PR, but is the intent to keep relocating those to the modules that own them as they gain @ConfigurationProperties classes? Worth capturing in #15469 so this file does not stay the central fallback by default.
| import java.lang.reflect.Field | ||
| import java.lang.reflect.Modifier | ||
|
|
||
| class ConfigurationMetadataTransformationSpec extends Specification { |
There was a problem hiding this comment.
Two feature methods for a 277-line transform leaves most of the logic that decides what gets published unexercised. The TestKit spec covers the Java/ASM half thoroughly, but the Groovy half is only tested through one happy-path class, so the branches below can change behaviour without any test noticing:
@Delegatefield exclusion, and thesetGrailsApplication/setMetaClassexclusions.collectSetterPropertieswalking superclasses and interfaces — a config class extending a base class and implementing a trait is the shape both view configurations actually have, and it is the source of most generated properties.constructorBoundPropertiesselection:@ConstructorBindingon one of several constructors, the "no no-arg constructor plus exactly one candidate" rule, and private/synthetic constructors being ignored.- A non-constant
prefixexpression, which makesaddPayloadbail out entirely and silently hands the class to the bytecode bean-scan path instead — worth pinning, since the two paths do not infer the same set. typeName/genericTypeNamefor arrays, wildcards (? extends/? super) and type variables.- The hand-rolled
toJson/escapeJson. A constant default containing a quote, backslash, newline or control character is written into a class-file string constant and parsed back byJsonSlurperin the Gradle task; if the escaping is ever wrong the failure surfaces as a JSON parse error during someone else's module build. - An interface annotated
@ConfigurationProperties(skipped by!node.interface), and a prefix-less@ConfigurationProperties.
Each is a few lines of parseClass in this spec, and they are what makes the mechanism safe to change later.
What this does
Closes #15469 for the five modules that already use
@ConfigurationProperties:grails-cachegrails-databindinggrails-views-gsongrails-views-markupgrails-web-url-mappings(CORS owner)Each publishes standard Spring Boot metadata at
META-INF/spring-configuration-metadata.jsonduring the module build. IDEs,config-report, and the Application Properties reference consume that resource.This is not Spring Boot's
spring-boot-configuration-processor. That approach was tried in #15566 and closed because Java annotation processors break incremental Groovy compilation. This PR uses a Grails-owned path instead:@ConfigurationPropertiesclass (no shared processor output).Migration details
spring-configuration-metadata.jsonrenamed toadditional-spring-configuration-metadata.json(overlay). Overlay values win for matching identities.8.0.x; generation now publishes bindable Markup Views properties.grails-web-coreintograils-web-url-mappingswithGrailsCorsConfiguration. Non-CORS web-core metadata stays hand-maintained ingrails-web-core.Generated defaults only include compile-time constants. Dynamic Groovy defaults are omitted unless an overlay supplies an authoritative value.
Compatibility check (against
origin/8.0.x)Re-verified on exact PR HEAD after regenerating metadata:
grails-web-core8.0.xfile)Each migrated jar contains exactly one standard metadata resource plus any curated additional metadata file.
Scope notes
@ConfigurationPropertiesmodules.DefaultSecurityConfig, generalapplication.groovymerging). Those remain later work.Verification
Local:
:grails-configuration-metadata:test+codeStyleConfigurationMetadataPluginSpec(clean, incremental, edit, delete, overlay, duplicate, immutable, generic-constructor safety)ConfigReportCommandSpec:grails-doc:publishGuide -x aggregateGroovydocCI on this PR: core builds (Linux/macOS), style/analysis/RAT/CodeQL/coverage, Forge, functional, security, Redis, MongoDB, and Hibernate suites are green. A few long jobs (Windows core, joint Groovy validation, selected functional reruns) were still finishing at description update time.
Commits
3-7. Per-module migrations (cache, databinding, gson, markup, URL mappings/CORS)
8-9. Docs reference inputs + guide prose